-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add view password functionality to login form #4714
Add view password functionality to login form #4714
Conversation
Functionality looks fine |
@@ -37,3 +60,20 @@ | |||
<%= link_to user_google_oauth2_omniauth_authorize_path, method: :post do %> | |||
<img src="../img/btn_google_signin_dark_focus_web@2x.png" alt="Sign in with Google"> | |||
<% end %> | |||
|
|||
<script> | |||
document.getElementById('toggle_password').onclick = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we package this into a Stimulus controller? That's the way we want to encourage sprinkling JavaScript into our pages. You can see an example here: https://github.com/rubyforgood/human-essentials/blob/8839e19eb67ed92fb4c96fa7a98beab2ddf9e09f/app/javascript/controllers/highchart_controller.js#L32-L32 and it's used
<button type="button" class="btn btn-sm btn-secondary" data-action="click->highchart#selectAll">Select All</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Dorner,
I've refactored the password toggle into a Stimulus controller as requested. It’s now encapsulated neatly in the password_visibility_controller.js, making our JS integration more structured.
Also, erb lint is failing - see above. |
Awesome, thanks! |
@princekumarg12: Your PR |
Resolves #4671
Description
This change implements an "eyeball" icon functionality on the login page, allowing users to toggle password visibility. This feature aims to improve user experience by enabling users to confirm their password entry, reducing login errors due to typos.
This solution balances usability with security, ensuring the password is hidden by default.
No additional dependencies are required for this change.
Type of change
How Has This Been Tested?
I implemented unit tests to verify the password toggle functionality: